Skip to content

Comments

Fix SEC-003 false positives in safe-outputs conformance check#17790

Merged
pelikhan merged 3 commits intomainfrom
copilot/fix-sec-003-false-positives
Feb 23, 2026
Merged

Fix SEC-003 false positives in safe-outputs conformance check#17790
pelikhan merged 3 commits intomainfrom
copilot/fix-sec-003-false-positives

Conversation

Copy link
Contributor

Copilot AI commented Feb 22, 2026

  • Scope SEC-003 check to only files with octokit. calls (like SEC-002)
  • Update grep pattern to include enforceArrayLimit, tryEnforceArrayLimit, and limit_enforcement_helpers
  • Skip test and utility files (test|parse|buffer|factory) before the octokit scope check
  • Verified: bash scripts/check-safe-outputs-conformance.sh reports [PASS] SEC-003: All handlers enforce max limits
Original prompt

This section details on the original issue you should resolve

<issue_title>[Safe Outputs Conformance] SEC-003: Max limit enforcement check produces 198 false positives due to overly broad scope and unrecognized enforcement pattern</issue_title>
<issue_description>### Conformance Check Failure

Check ID: SEC-003
Severity: MEDIUM
Category: Security / Check Design

Problem Description

The SEC-003 check in scripts/check-safe-outputs-conformance.sh currently flags 198 files as potentially not enforcing max limits. This high count is caused by two compounding issues:

  1. Overly broad file scope: The check iterates over all *.cjs files in actions/setup/js/, which includes fuzz test harnesses, utility modules, sanitizers, MCP transport files, constants, and other non-handler files that have no responsibility for enforcing max limits.

  2. Unrecognized enforcement pattern: The project's actual max limit enforcement is implemented via enforceArrayLimit() and tryEnforceArrayLimit() in actions/setup/js/limit_enforcement_helpers.cjs. The SEC-003 check pattern (\.length.*>.*\.max\|enforceMaxLimit\|checkLimit\|max.*exceeded) does not match these function names, so any handler that correctly delegates to the shared limit_enforcement_helpers.cjs module is still flagged.

The result is that the check cannot distinguish files that correctly use the shared enforcement helper from files that genuinely lack any limit enforcement. This makes the check ineffective for its intended security purpose.

Affected Components

  • Check script: scripts/check-safe-outputs-conformance.sh (lines 106–123, check_max_limits function)
  • Enforcement module: actions/setup/js/limit_enforcement_helpers.cjs (exports enforceArrayLimit, tryEnforceArrayLimit)
  • Flagged files (sample): All 198 files in actions/setup/js/ excluding those matching test|parse|buffer|factory — including utility, sanitizer, MCP transport, fuzz harness, and message files
View representative false-positive categories
Category Example files
Fuzz harnesses fuzz_markdown_code_region_balancer_harness.cjs, fuzz_mentions_harness.cjs, fuzz_sanitize_incoming_text_harness.cjs
Sanitizers sanitize_content.cjs, sanitize_output.cjs, sanitize_title.cjs
MCP transport mcp_http_transport.cjs, mcp_server_core.cjs, mcp_logger.cjs
Messages / UI messages.cjs, messages_footer.cjs, messages_staged.cjs
Utilities constants.cjs, error_codes.cjs, is_truthy.cjs, git_helpers.cjs
The enforcement helper itself limit_enforcement_helpers.cjs

Current Behavior

# Check incorrectly flags all non-excluded .cjs files:
for handler in actions/setup/js/*.cjs; do
    [[ "$handler" =~ (test|parse|buffer|factory) ]] && continue
    if ! grep -q "\.length.*>.*\.max\|enforceMaxLimit\|checkLimit\|max.*exceeded" "$handler"; then
        log_medium "SEC-003: $handler may not enforce max limits"
    fi
done

Files that correctly require('./limit_enforcement_helpers') and call enforceArrayLimit() are still flagged because the grep pattern does not recognize those function names.

Expected Behavior

The SEC-003 check should:

  1. Only target files that are actual safe output handlers (e.g., files that make GitHub API calls via octokit).
  2. Recognize the project's standard enforcement functions: enforceArrayLimit, tryEnforceArrayLimit, as well as any require.*limit_enforcement_helpers import.

Remediation Steps

This task can be assigned to a Copilot coding agent with the following steps:

  1. Scope the check to only files that actually perform GitHub API operations. A reliable proxy: files that contain octokit. calls (consistent with how SEC-002 scopes its check).
  2. Update the grep pattern to include the project's actual enforcement functions:
    grep -qE "\.length.*>.*\.max|enforceMaxLimit|checkLimit|max.*exceeded|enforceArrayLimit|tryEnforceArrayLimit|limit_enforcement_helpers" "$handler"
  3. Run the updated check and resolve any genuine remaining failures (files that perform API operations and do not use the shared enforcement helper for array parameters).

Verification

After remediation, verify the fix by running:

bash scripts/check-safe-outputs-conformance.sh

SEC-003 should report significantly fewer failures, limited to files with genuine missing enforcement.

References

  • Safe Outputs Specification: docs/src/content/docs/reference/safe-outputs-specification.md
  • Conformance Checker: scripts/check-safe-outputs-conformance.sh (lines 106–123)
  • Limit Enforcement Module: actions/setup/js/limit_enforcement_helpers.cjs
  • Run ID: §22281286232
  • Date: 2026-02-22

Generated by [Daily Safe Outputs Confor...


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

…t patterns

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix false positives in SEC-003 max limit enforcement check Fix SEC-003 false positives in safe-outputs conformance check Feb 22, 2026
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
@pelikhan pelikhan marked this pull request as ready for review February 23, 2026 00:09
Copilot AI review requested due to automatic review settings February 23, 2026 00:09
@pelikhan pelikhan merged commit 6dca926 into main Feb 23, 2026
33 checks passed
@pelikhan pelikhan deleted the copilot/fix-sec-003-false-positives branch February 23, 2026 00:09
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR attempts to fix false positives in the SEC-003 conformance check by scoping it to only check files that perform GitHub API operations and updating the grep pattern to recognize the project's actual limit enforcement functions.

Changes:

  • Added a filter to only check files containing octokit. calls (lines 114-117)
  • Updated the grep pattern to include enforceArrayLimit, tryEnforceArrayLimit, and limit_enforcement_helpers (line 120)
  • Moved test/utility file exclusions before the octokit scope check (line 112)
Comments suppressed due to low confidence (1)

scripts/check-safe-outputs-conformance.sh:120

  • The grep pattern updates on line 120 correctly add the enforcement function names (enforceArrayLimit, tryEnforceArrayLimit, limit_enforcement_helpers), but they will never be evaluated because line 115 filters out all handler files. Since no files match the pattern octokit. (handlers use github.rest. and github.graphql instead), the check will skip all files and incorrectly report a pass. Additionally, the existing SEC-002 check has the same issue on line 90.
        if ! grep -qE "\.length.*>.*\.max|enforceMaxLimit|checkLimit|max.*exceeded|enforceArrayLimit|tryEnforceArrayLimit|limit_enforcement_helpers" "$handler"; then

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

# Check if handler enforces max limits
if ! grep -q "\.length.*>.*\.max\|enforceMaxLimit\|checkLimit\|max.*exceeded" "$handler"; then
# Only check files that perform GitHub API operations
if ! grep -q "octokit\." "$handler"; then
Copy link

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The pattern octokit. does not match the actual GitHub API usage pattern in this codebase. Handler files use github.rest. and github.graphql (where github is a global variable from @actions/github-script), not octokit.. This means the scope filter will match zero files, causing SEC-003 to report a false pass even though it's not checking any handlers. The same issue exists in SEC-002 (line 90). The pattern should be changed to match actual API calls, such as github\.rest\.|github\.graphql or simply \.rest\.|\.graphql.

This issue also appears on line 120 of the same file.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants